Use Monaco Editor and save code in submitpage.php when edit the content#989
Use Monaco Editor and save code in submitpage.php when edit the content#989def-WA2025 wants to merge 13 commits into
Conversation
Reviewer's GuideIntegrates Monaco Editor as the primary code editor on submitpage.php (with local persistence and keyboard shortcuts), adds a CodeMirror-compatible shim backed by Monaco, updates error message code snippets to use Monaco when available, and fixes a minor height fallback bug in the Monaco adapter, with corresponding version bumps. Sequence diagram for Monaco editor initialization on submitpage.phpsequenceDiagram
participant Browser as Browser
participant main as main
participant createMonacoEditor as createMonacoEditor
participant ensureMonaco as ensureMonaco
participant Monaco as monaco.editor
participant FallbackTA as CodeInput_textarea
Browser->>main: main()
main->>main: detect /submitpage.php
main->>createMonacoEditor: createMonacoEditor('MonacoEditor', options)
createMonacoEditor->>ensureMonaco: ensureMonaco()
ensureMonaco-->>createMonacoEditor: monaco ready
createMonacoEditor->>Monaco: monaco.editor.create(container, ...)
Monaco-->>createMonacoEditor: editor
createMonacoEditor-->>main: adapter (CodeMirrorElement)
main->>Monaco: _monacoEditor.addCommand(Ctrl+Enter)
main->>Monaco: _monacoEditor.addCommand(Ctrl+Space)
alt Monaco load fails
createMonacoEditor--x main: throw Error
main->>FallbackTA: show textarea fallback
main->>main: build fallback adapter
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Deploying xmoj-script-dev-channel with
|
| Latest commit: |
ca1cdbe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2d8054dd.xmoj-script-dev-channel.pages.dev |
| Branch Preview URL: | https://def-wa2025-monaco-editor-and.xmoj-script-dev-channel.pages.dev |
There was a problem hiding this comment.
Hey - I've found 2 security issues, 1 other issue, and left some high level feedback:
Security issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
document.querySelector("body > div > div.mt-3").innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- On submitpage.php the hidden #CodeInput textarea is no longer bound to the editor as it was with CodeMirror.fromTextArea; make sure you copy the Monaco editor content back into #CodeInput (e.g., in the submit handler) so the backend still receives the code.
- In the freopen snippet error block you always append an empty div (codeHost) to #ErrorMessage and, in the fallback path, also append a
directly to #ErrorMessage; consider either rendering the fallback
inside codeHost or skipping codeHost creation when Monaco is unavailable to avoid redundant markup.
- The CodeMirror shim exposes _monacoEditor and callers reach into that to add commands; consider exposing a small public API (e.g., addCommand/triggerSuggest) on the adapter instead of using the private _monacoEditor field to keep the abstraction boundary cleaner.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On submitpage.php the hidden #CodeInput textarea is no longer bound to the editor as it was with CodeMirror.fromTextArea; make sure you copy the Monaco editor content back into #CodeInput (e.g., in the submit handler) so the backend still receives the code.
- In the freopen snippet error block you always append an empty div (codeHost) to #ErrorMessage and, in the fallback path, also append a <pre> directly to #ErrorMessage; consider either rendering the fallback <pre> inside codeHost or skipping codeHost creation when Monaco is unavailable to avoid redundant markup.
- The CodeMirror shim exposes _monacoEditor and callers reach into that to add commands; consider exposing a small public API (e.g., addCommand/triggerSuggest) on the adapter instead of using the private _monacoEditor field to keep the abstraction boundary cleaner.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="374-383" />
<code_context>
+ shim.MergeView = function(container, options) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** MergeView hardcodes the language to C++, ignoring any mode/language passed in options.
The shim currently instantiates both models with language `'cpp'` and ignores `options.mode` (or any language option), which changes behavior compared to the CodeMirror-based MergeView for non-C++ content. Please derive the Monaco language from the provided mode (similar to `shim`/`createMonacoEditor`, mapping known CodeMirror modes and defaulting to C++) so other languages still get appropriate syntax handling.
</issue_to_address>
### Comment 2
<location path="XMOJ.user.js" line_range="3633-3638" />
<code_context>
document.querySelector("body > div > div.mt-3").innerHTML = `<center class="mb-3">` + `<h3>提交代码</h3>` + (SearchParams.get("id") != null ? `题目<span class="blue">${Number(SearchParams.get("id"))}</span>` : `比赛<span class="blue">${Number(SearchParams.get("cid")) + `</span> 题目<span class="blue">` + String.fromCharCode(65 + parseInt(SearchParams.get("pid")))}</span>`) + `</center>
<div id="MonacoEditor" style="width:100%; height:400px;"></div>
<textarea id="CodeInput" style="display:none"></textarea>
<center class="mt-3">
<input id="enable_O2" name="enable_O2" type="checkbox"><label for="enable_O2">打开O2开关</label>
<br>
<input id="Submit" class="btn btn-info mt-2" type="button" value="提交">
<div id="ErrorElement" class="mt-2" style="display: none; text-align: left; padding: 10px;">
<div id="ErrorMessage" style="white-space: pre; background-color: rgba(0, 0, 0, 0.1); padding: 10px; border-radius: 5px;"></div>
<button id="PassCheck" class="btn btn-outline-secondary mt-2" style="display: none">强制提交</button>
</div>
</center>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 3
<location path="XMOJ.user.js" line_range="3633-3638" />
<code_context>
document.querySelector("body > div > div.mt-3").innerHTML = `<center class="mb-3">` + `<h3>提交代码</h3>` + (SearchParams.get("id") != null ? `题目<span class="blue">${Number(SearchParams.get("id"))}</span>` : `比赛<span class="blue">${Number(SearchParams.get("cid")) + `</span> 题目<span class="blue">` + String.fromCharCode(65 + parseInt(SearchParams.get("pid")))}</span>`) + `</center>
<div id="MonacoEditor" style="width:100%; height:400px;"></div>
<textarea id="CodeInput" style="display:none"></textarea>
<center class="mt-3">
<input id="enable_O2" name="enable_O2" type="checkbox"><label for="enable_O2">打开O2开关</label>
<br>
<input id="Submit" class="btn btn-info mt-2" type="button" value="提交">
<div id="ErrorElement" class="mt-2" style="display: none; text-align: left; padding: 10px;">
<div id="ErrorMessage" style="white-space: pre; background-color: rgba(0, 0, 0, 0.1); padding: 10px; border-radius: 5px;"></div>
<button id="PassCheck" class="btn btn-outline-secondary mt-2" style="display: none">强制提交</button>
</div>
</center>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `document.querySelector("body > div > div.mt-3").innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@copilot fix all |
…ck editor, package.json version
…nt, fallback interface stubs
Fixed all AI-detected bugs in commits
|
Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net>
typeof h === 'number' check in Monaco adapter setSize
typeof h === 'number' check in Monaco adapter setSize|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
CodeMirrorElementvariable name is now misleading since it always wraps Monaco or a textarea fallback; consider renaming it (and related comments) to something editor-agnostic to avoid future confusion. - The submit page setup logic (header construction, editor creation, fallback handling, keyboard shortcuts) is quite long and nested in
main; extracting this into a dedicated helper function would improve readability and make future changes to the submit page easier. - There are several repeated theme decisions using
UtilityEnabled("DarkMode") ? 'vs-dark' : 'vs'; consider centralizing this into a small helper to ensure consistency and simplify future theme adjustments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `CodeMirrorElement` variable name is now misleading since it always wraps Monaco or a textarea fallback; consider renaming it (and related comments) to something editor-agnostic to avoid future confusion.
- The submit page setup logic (header construction, editor creation, fallback handling, keyboard shortcuts) is quite long and nested in `main`; extracting this into a dedicated helper function would improve readability and make future changes to the submit page easier.
- There are several repeated theme decisions using `UtilityEnabled("DarkMode") ? 'vs-dark' : 'vs'`; consider centralizing this into a small helper to ensure consistency and simplify future theme adjustments.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net>
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@copilot fix new bugs found by ai |
Fixed all three open AI-detected bugs in commit
|
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
|
@cubic-dev-ai review |
@def-WA2025 I have started the AI code review. It will take a few minutes to complete. |
Inside the Monaco adapter's
setSize, thecatchfallback for theh === 'auto'branch contained a dead condition:typeof h === 'number'is alwaysfalsebecausehmust be'auto'to reach that branch.Change
XMOJ.user.jsline 321 — replace the unreachable ternary with a direct'80px'fallback, consistent with theMath.max(80, …)floor used in the happy path above it:Summary by Sourcery
Replace the submit page code editor with a Monaco-based implementation that persists code locally and update supporting editor utilities and snippets.
New Features:
Bug Fixes:
Enhancements:
Build: